Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#115 Device orientation for isFlat #116

Closed
wants to merge 7 commits into from

Conversation

CmdrDats
Copy link

Solves #115 - I'm not sure why the tooling removed the zoomRatio though? Should I manually revert that?

Pull request checklist

Please check if your PR fulfills the following requirements:

  • The changes have been tested successfully.
  • A changeset has been created (npm run changeset).
  • I have read and followed the pull request guidelines.

@CmdrDats CmdrDats changed the title #115 Device orientation for isFlat + screenSize on barcode reading. #115 Device orientation for isFlat Dec 28, 2023
@@ -12,7 +12,7 @@ public class BarcodeScannerHelper {
let screenSize: CGRect = UIScreen.main.bounds
let imageWidth = imageSize.width
let imageHeight = imageSize.height
let isPortrait = UIDevice.current.orientation == .portrait || UIDevice.current.orientation == .portraitUpsideDown
let isPortrait = UIDevice.current.orientation.isPortrait || UIDevice.current.orientation.isFlat
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't isFlat also be true if the device was in a landscape orientation before so that the UI is still in landscape mode or am I misunderstanding isFlat?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ye, that's why I'm uncomfortable with this 'solution' - works for my case (I only support portrait mode UI), but then breaks the landscape case. The method for figuring out actual UI orientation seems to need to run on the main UI thread (I'm not quite sure how to sample that here, seeing as this is my first time ever touching swift 🙈 )

An alternative not-really-a-solution is to send through the orientation and let the underlying app decide how to correct, but that seems iffy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method for figuring out actual UI orientation seems to need to run on the main UI thread (I'm not quite sure how to sample that here, seeing as this is my first time ever touching swift 🙈 )

You can take a look at our Screen Orientation plugin: https://github.com/capawesome-team/capacitor-plugins/blob/main/packages/screen-orientation/ios/Plugin/ScreenOrientation.swift#L73:L79
Maybe you can find a better solution.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked around, and the most accurate thing I can see is to bubble it through from the interfaceOrientation in layoutSubviews https://github.com/capawesome-team/capacitor-mlkit/pull/116/files#diff-8b77d4c809bf757f8fef70caf790b8b021e201d15987c2d724bf0af350b14b97R113

This works correctly in testing on all orientations now, except that I needed to add 1- on the landscape coords for it to map correctly in my testing - I'm not sure why this is?

@CmdrDats
Copy link
Author

@robingenz any further thoughts on this one?

@robingenz
Copy link
Member

@robingenz any further thoughts on this one?

Sorry for my late response. Your PR is still on my to-do list. I will get back to you soon.

@CmdrDats
Copy link
Author

amazing, thank you!

@robingenz
Copy link
Member

@CmdrDats Here you can find the current dev build: #128 (comment)

Feel free to give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants